-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Entity Analytics] Use docLinks service for documentation links #172172
Conversation
import { LEARN_MORE } from '../../../../overview/components/entity_analytics/risk_score/translations'; | ||
import { useKibana } from '../../../../common/lib/kibana'; | ||
|
||
const useLearnMoreLinkForEntity = (riskScoreEntity?: RiskScoreEntity) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily a suggestion, more of a question:
Do you think there would be value in adding a test case for this, now that we're using more dynamic links instead of them just being hardcoded? I assume it would involve mocking the docLinks service, so not sure how much value we'd get, but thought I'd ask!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did debate testing it, but I concluded the tests would be too tied to the implementation to give much value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to update the URL's to include the domain and base path, unless it's just a local limitation on my side!
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @hop-dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doc link service additions LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validated that the changes requested were made, looks great!
Summary
Using the docLinks service means documentation links will always point to the correct version of the docs for the current Kibana version, not just the latest docs.